Thanks for the contribution! Please see https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins for the process to make a multi-package PR, so that our CI can run tests.

Thanks for the link, I missed that!

I might be missing something very obvious here, but I'm not sure how to update so that the remaining checks will complete. From what I understand it fails due to a cached version of video_player_platform_interface instead of using the path-based dependency:

 Compilation failed for testPath=/b/s/w/ir/x/w/packages/packages/camera/camera/example/test/main_test.dart: ../../../video_player/video_player/lib/video_player.dart:440:34: Error: The method 'setAllowBackgroundPlayback' isn't defined for the class 'VideoPlayerPlatform'.
   - 'VideoPlayerPlatform' is from 'package:video_player_platform_interface/video_player_platform_interface.dart' ('../../../../../.pub-cache/hosted/pub.dev/video_player_platform_interface-6.3.0/lib/video_player_platform_interface.dart').
  Try correcting the name to the name of an existing method, or defining a method named 'setAllowBackgroundPlayback'.
        await _videoPlayerPlatform.setAllowBackgroundPlayback(

I can get the camera example to run locally, but only if I use direct dependencies instead of overridden dependencies.

What's the best approach here? Should I just remove the path-based dependencies for the camera example or is there another way to resolve it?

@stuartmorgan-g It looks like dependency_overrides doesn't work for transitive dependencies: https://dart.dev/tools/pub/dependencies#dependency-overrides

Only the dependency overrides in a package's own pubspec are considered during package resolution. Dependency overrides inside any depended-on packages are ignored.

@ArvidNy You should be able to fix this by manually adding the rest of the dependencies to the failing plugins. They should look like:

dependency_overrides:
  video_player: {path: ../../../../packages/video_player/video_player}
  video_player_android: {path: ../../../../packages/video_player/video_player_android}
  video_player_avfoundation: {path: ../../../../packages/video_player/video_player_avfoundation}
  video_player_platform_interface: {path: ../../../../packages/video_player/video_player_platform_interface}
  video_player_web: {path: ../../../../packages/video_player/video_player_web}

Let me know if that works.

@stuartmorgan-g It looks like dependency_overrides doesn't work for transitive dependencies

Yes, the tooling should account for that though.

@ArvidNy You should be able to fix this by manually adding the rest of the dependencies to the failing plugins.

That shouldn't have been necessary. What command did you run, exactly, to generate what's in the PR now?

This is the command I ran:

 dart run script/tool/bin/flutter_plugin_tools.dart make-deps-path-based --target-dependencies=video_player_platform_interface,video_player_android,video_player_avfoundation,video_player,video_player_web

Oh, and manually adding the rest of the dependency overrides does seem to resolve the issue, at least locally. So I can push that to resolve the PR tests if there's no other obvious fix for it.

I figured you would want consistency in the code. On Android we don't always want to use wake lock so we need to know when the option is true on the Flutter side. The option allowBackgroundPlayback was missing on the Java end, so I traced how mixWithOthers was set and used the same format to be able to use it like this:

I'm not sure that's better, but it allows for flexibility in case Apple would change anything in the future and we need to know when the option is set on another platform as well.

That being said, I'm all for reducing the complexity of the PR if that's preferable. Is it better to only add the method to the Android implementation then and not the VideoPlayerPlatform abstract class to avoid having to override it on all platforms?

I figured you would want consistency in the code.

We want each implementation to be written in the way that makes sense for that implementation.

I'm not sure that's better, but it allows for flexibility in case Apple would change anything in the future and we need to know when the option is set on another platform as well.

The Dart code and native code in question are part of the same package. There is no case where have the ability to change the native code in a package but not the Dart code in the same package, so I'm not sure what flexibility you are referring to.

That being said, I'm all for reducing the complexity of the PR if that's preferable. Is it better to only add the method to the Android implementation then and not the VideoPlayerPlatform abstract class to avoid having to override it on all platforms?

Making a no-op implementation in Dart, avoiding unnecessary round trips to the host side, does not require removing the cross-platform abstraction. @override Future<void> setAllowBackgroundPlayback(bool allowBackgroundPlayback) async {} is valid code that has the same effect as what the PR currently does on iOS, but with far less overhead.

What the cross-platform abstraction should or shouldn't be is a separate question of behavior, not implementation. That's a question of what happens on other platforms. For instance, what is the expected behavior of setAllowBackgroundPlayback(true) and setAllowBackgroundPlayback(false) on iOS? Currently this PR doesn't do the right things for at least one of those, it seems. See https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-support for discussion of that aspect.

Oh, you're right. I didn't think about that fact that the communication with the host side was unnecessary there and I didn't make that connection from your previous message – sorry about that. I've just removed the no-op on the native side for iOS and merged the latest changes from the PR you mentioned.

For Android, the setWakeMode still needs to be called, so the other PR only partly resolved the background playback issue.

@ArvidNy Are you still planning to address the remaining review feedback?

Sure, I plan on resolving the feedback I got so this can be merged. I've been busy with other projects for the past months, but I hope I'll get around to it soon.

I've now created a PR for only the platform interface package changes: #10054. I made the changes that the Gemini bot suggested if you want to review the new PR now.